Skip to content

created new erc20 token#78

Closed
tusharrrr1 wants to merge 4 commits intoTheSoftwareDevGuild:mainfrom
tusharrrr1:feat/new-erc-20-token
Closed

created new erc20 token#78
tusharrrr1 wants to merge 4 commits intoTheSoftwareDevGuild:mainfrom
tusharrrr1:feat/new-erc-20-token

Conversation

@tusharrrr1
Copy link

fixes #65 Added a complete ERC20 token system for rewarding Guild contributions with GitHub integration.

Copy link
Contributor

@joelamouche joelamouche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the submission!
Here is a first review.
Don't forget to update tests and specs with your changes

console.log(projectId);

// Fallback project ID for development
const fallbackProjectId = "2b3c4d5e6f7g8h9i0j1k2l3m4n5o6p7q";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this your ID? You should probably remove this. The dev ids can be set in dev env

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I will pass these ids into env

@@ -0,0 +1,205 @@
# TheGuild Contribution Token (TGC)

A standard ERC20 token designed for rewarding contributions to The Guild community. The token features owner-controlled minting, batch operations for gas efficiency, and a maximum supply cap.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be no max supply cap


### GitHub Issue Tracking Workflow

1. **Contributor completes task**: GitHub issue #123 is closed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update issue number

require(recipients.length == amounts.length, "TGC: arrays length mismatch");
require(recipients.length > 0, "TGC: empty arrays");

uint256 totalAmount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totalAmount is not very useful imo.
Instead, please include distributionId as per described in the specs of the parent ticket: batchMint uses a distributionId as input and checks that distributionId is not in a mapping of already distributed distributionIds

require(recipients.length == reasons.length, "TGC: recipients/reasons length mismatch");
require(recipients.length > 0, "TGC: empty arrays");

uint256 totalAmount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

uint256 totalAmount = 0;

for (uint256 i = 0; i < recipients.length; i++) {
require(recipients[i] != address(0), "TGC: cannot mint to zero address");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so here you could reuse mintWithReason

require(totalSupply() + amount <= MAX_SUPPLY, "TGC: would exceed max supply");

_mint(to, amount);
emit Mint(to, amount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not necessary since the erc20 will emit a transfer event

TheGuildContributionToken token = deployToken(initialOwner);

if (recipients.length > 0) {
vm.startBroadcast();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the broadcast surround the entire loop?

/// @param csvLine The CSV line in format "address,amount"
/// @return recipient The parsed address
/// @return amount The parsed amount
function parseLine(string memory csvLine) internal pure returns (address recipient, uint256 amount) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this used?

uint256[] memory amounts = new uint256[](3);

// Example data - replace with actual addresses and amounts
recipients[0] = 0x1234567890123456789012345678901234567890;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be loaded from CSV

@joelamouche
Copy link
Contributor

@tusharrrr1 did you see my review?

@joelamouche
Copy link
Contributor

Sorry @tusharrrr1 but with no update and now progress on this I have to close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a contribution token to our foundry repository

2 participants

Comments